widget: fix class private data usage to be _init() safe

Before this commit, adding GtkWidgetAction to class private data would
require copying the actions to each subclass as they were built or
modified. This was convenient in that it is a sort of "copy on write"
semantic.

However, due to the way that GTypeInstance works with base _init()
functions, the "g_class" pointer in GTypeInstance is updated as each
_init() function is called. That means you cannot access the subclasses
class private data, but only the parent class private data.

If instead we use a singly linked list of GtkWidgetAction, each subclass
has their own "head" yet all subclasses share the tail of the
GtkWidgetAction chain.

This creates one bit of complexity though. You need a stable way to know
which "bit" is the "enabled" bit of the action so we can track enabled
GAction state. That is easily solved by calculating the distance to the
end of the chain for a given action so that base classes sort ahead of
subclasses. Since the parent class always knows its parent's actions, the
position is stable.

A new dynamic bitarray helper also helps us avoid allocations in all the
current cases (up to 64 actions per widget) and dynamically switches to
malloc if that is to ever be exceeded.

diff --git a/gtk/gtkactionmuxer.c b/gtk/gtkactionmuxer.c
index 521f77e6f0118397372d29b39813c4d51b9710be..303d26e7387f4f46abba7b5a71b880b4bfcc2012 100644 (file)
--- a/gtk/gtkactionmuxer.c
+++ b/gtk/gtkactionmuxer.c
@@ -23,9 +23,10 @@
 
 #include "gtkactionobservableprivate.h"
 #include "gtkactionobserverprivate.h"
+#include "gtkbitmaskprivate.h"
 #include "gtkintl.h"
 #include "gtkmarshalers.h"
-#include "gtkwidget.h"
+#include "gtkwidgetprivate.h"
 #include "gsettings-mapping.h"
 
 #include <string.h>
@@ -73,8 +74,8 @@ struct _GtkActionMuxer
   GtkActionMuxer *parent;
 
   GtkWidget *widget;
-  GPtrArray *widget_actions;
-  gboolean *widget_actions_enabled;
+
+  GtkBitmask *widget_actions_disabled;
 };
 
 G_DEFINE_TYPE_WITH_CODE (GtkActionMuxer, gtk_action_muxer, G_TYPE_OBJECT,
@@ -86,7 +87,6 @@ enum
   PROP_0,
   PROP_PARENT,
   PROP_WIDGET,
-  PROP_WIDGET_ACTIONS,
   NUM_PROPERTIES
 };
 
@@ -109,6 +109,19 @@ typedef struct
   gulong        handler_ids[4];
 } Group;
 
+static inline guint
+get_action_position (GtkWidgetAction *action)
+{
+  guint slot;
+  /* We use the length of @action to the end of the chain as the slot so that
+   * we have stable positions for any class or it's subclasses. Doing so helps
+   * us avoid having mutable arrays in the class data as we will not have
+   * access to the ClassPrivate data during instance _init() functions.
+   */
+  for (slot = 0; action->next != NULL; slot++, action = action->next) {}
+  return slot;
+}
+
 static void
 gtk_action_muxer_append_group_actions (const char *prefix,
                                        Group      *group,
@@ -143,15 +156,14 @@ gtk_action_muxer_list_actions (GActionGroup *action_group)
       const char *prefix;
       Group *group;
 
-      if (muxer->widget_actions)
+      if (muxer->widget)
         {
-          int i;
+          GtkWidgetClass *klass = GTK_WIDGET_GET_CLASS (muxer->widget);
+          GtkWidgetClassPrivate *priv = klass->priv;
+          GtkWidgetAction *action;
 
-          for (i = 0; i < muxer->widget_actions->len; i++)
-            {
-              GtkWidgetAction *action = g_ptr_array_index (muxer->widget_actions, i);
-              g_hash_table_add (actions, g_strdup (action->name));
-            }
+          for (action = priv->actions; action; action = action->next)
+            g_hash_table_add (actions, g_strdup (action->name));
         }
 
       g_hash_table_iter_init (&iter, muxer->groups);
@@ -217,22 +229,27 @@ gtk_action_muxer_action_enabled_changed (GtkActionMuxer *muxer,
                                          const gchar    *action_name,
                                          gboolean        enabled)
 {
+  GtkWidgetAction *iter;
   Action *action;
   GSList *node;
 
-  if (muxer->widget_actions)
+  if (muxer->widget)
     {
-      int i;
-      for (i = 0; i < muxer->widget_actions->len; i++)
+      GtkWidgetClass *klass = GTK_WIDGET_GET_CLASS (muxer->widget);
+      GtkWidgetClassPrivate *priv = klass->priv;
+
+      for (iter = priv->actions; iter; iter = iter->next)
         {
-          GtkWidgetAction *a = g_ptr_array_index (muxer->widget_actions, i);
-          if (strcmp (a->name, action_name) == 0)
+          if (strcmp (action_name, iter->name) == 0)
             {
-              muxer->widget_actions_enabled[i] = enabled;
+              guint position = get_action_position (iter);
+              muxer->widget_actions_disabled =
+                _gtk_bitmask_set (muxer->widget_actions_disabled, position, !enabled);
               break;
             }
         }
     }
+
   action = g_hash_table_lookup (muxer->observed_actions, action_name);
   for (node = action ? action->watchers : NULL; node; node = node->next)
     gtk_action_observer_action_enabled_changed (node->data, GTK_ACTION_OBSERVABLE (muxer), action_name, enabled);
@@ -517,21 +534,21 @@ prop_action_notify (GObject    *object,
                     gpointer    user_data)
 {
   GtkActionMuxer *muxer = user_data;
-  int i;
+  GtkWidgetClass *klass = GTK_WIDGET_GET_CLASS (muxer->widget);
+  GtkWidgetClassPrivate *priv = klass->priv;
   GtkWidgetAction *action = NULL;
   GVariant *state;
 
   g_assert ((GObject *)muxer->widget == object);
 
-  for (i = 0; i < muxer->widget_actions->len; i++)
+  for (action = priv->actions; action; action = action->next)
     {
-      action = g_ptr_array_index (muxer->widget_actions, i);
       if (action->pspec == pspec)
         break;
-      action = NULL;
     }
 
   g_assert (action != NULL);
+  g_assert (action->pspec == pspec);
 
   state = prop_action_get_state (muxer->widget, action);
   gtk_action_muxer_action_state_changed (muxer, action->name, state);
@@ -541,14 +558,20 @@ prop_action_notify (GObject    *object,
 static void
 prop_actions_connect (GtkActionMuxer *muxer)
 {
-  int i;
+  GtkWidgetClassPrivate *priv;
+  GtkWidgetAction *action;
+  GtkWidgetClass *klass;
+
+  if (!muxer->widget)
+    return;
 
-  if (!muxer->widget || !muxer->widget_actions)
+  klass = GTK_WIDGET_GET_CLASS (muxer->widget);
+  priv = klass->priv;
+  if (!priv->actions)
     return;
 
-  for (i = 0; i < muxer->widget_actions->len; i++)
+  for (action = priv->actions; action; action = action->next)
     {
-      GtkWidgetAction *action = g_ptr_array_index (muxer->widget_actions, i);
       char *detailed;
 
       if (!action->pspec)
@@ -572,41 +595,46 @@ gtk_action_muxer_query_action (GActionGroup        *action_group,
                                GVariant           **state)
 {
   GtkActionMuxer *muxer = GTK_ACTION_MUXER (action_group);
+  GtkWidgetAction *action;
   Group *group;
   const gchar *unprefixed_name;
 
-  if (muxer->widget_actions)
+  if (muxer->widget)
     {
-      int i;
+      GtkWidgetClass *klass = GTK_WIDGET_GET_CLASS (muxer->widget);
+      GtkWidgetClassPrivate *priv = klass->priv;
 
-      for (i = 0; i < muxer->widget_actions->len; i++)
+      for (action = priv->actions; action; action = action->next)
         {
-          GtkWidgetAction *action = g_ptr_array_index (muxer->widget_actions, i);
-          if (strcmp (action->name, action_name) == 0)
-            {
-              if (enabled)
-                *enabled = muxer->widget_actions_enabled[i];
-              if (parameter_type)
-                *parameter_type = action->parameter_type;
-              if (state_type)
-                *state_type = action->state_type;
+          guint position;
 
-              if (state_hint)
-                *state_hint = NULL;
-              if (state)
-                *state = NULL;
+          if (strcmp (action->name, action_name) != 0)
+            continue;
 
-              if (action->pspec)
-                {
-                  if (state)
-                    *state = prop_action_get_state (muxer->widget, action);
-                  if (state_hint)
-                    *state_hint = prop_action_get_state_hint (muxer->widget, action);
-                }
+          position = get_action_position (action);
+
+          if (enabled)
+            *enabled = !_gtk_bitmask_get (muxer->widget_actions_disabled, position);
+          if (parameter_type)
+            *parameter_type = action->parameter_type;
+          if (state_type)
+            *state_type = action->state_type;
+
+          if (state_hint)
+            *state_hint = NULL;
+          if (state)
+            *state = NULL;
 
-              return TRUE;
+          if (action->pspec)
+            {
+              if (state)
+                *state = prop_action_get_state (muxer->widget, action);
+              if (state_hint)
+                *state_hint = prop_action_get_state_hint (muxer->widget, action);
             }
-       }
+
+          return TRUE;
+        }
     }
 
   group = gtk_action_muxer_find_group (muxer, action_name, &unprefixed_name);
@@ -629,19 +657,22 @@ gtk_action_muxer_activate_action (GActionGroup *action_group,
                                   GVariant     *parameter)
 {
   GtkActionMuxer *muxer = GTK_ACTION_MUXER (action_group);
-  Group *group;
   const gchar *unprefixed_name;
+  Group *group;
 
-  if (muxer->widget_actions)
+  if (muxer->widget)
     {
-      int i;
+      GtkWidgetClass *klass = GTK_WIDGET_GET_CLASS (muxer->widget);
+      GtkWidgetClassPrivate *priv = klass->priv;
+      GtkWidgetAction *action;
 
-      for (i = 0; i < muxer->widget_actions->len; i++)
+      for (action = priv->actions; action; action = action->next)
         {
-          GtkWidgetAction *action = g_ptr_array_index (muxer->widget_actions, i);
           if (strcmp (action->name, action_name) == 0)
             {
-              if (muxer->widget_actions_enabled[i])
+              guint position = get_action_position (action);
+
+              if (!_gtk_bitmask_get (muxer->widget_actions_disabled, position))
                 {
                   if (action->activate)
                     action->activate (muxer->widget, action->name, parameter);
@@ -668,16 +699,17 @@ gtk_action_muxer_change_action_state (GActionGroup *action_group,
                                       GVariant     *state)
 {
   GtkActionMuxer *muxer = GTK_ACTION_MUXER (action_group);
-  Group *group;
+  GtkWidgetAction *action;
   const gchar *unprefixed_name;
+  Group *group;
 
-  if (muxer->widget_actions)
+  if (muxer->widget)
     {
-      int i;
+      GtkWidgetClass *klass = GTK_WIDGET_GET_CLASS (muxer->widget);
+      GtkWidgetClassPrivate *priv = klass->priv;
 
-      for (i = 0; i < muxer->widget_actions->len; i++)
+      for (action = priv->actions; action; action = action->next)
         {
-          GtkWidgetAction *action = g_ptr_array_index (muxer->widget_actions, i);
           if (strcmp (action->name, action_name) == 0)
             {
               if (action->pspec)
@@ -803,10 +835,9 @@ gtk_action_muxer_finalize (GObject *object)
   if (muxer->primary_accels)
     g_hash_table_unref (muxer->primary_accels);
 
-  g_free (muxer->widget_actions_enabled);
+  _gtk_bitmask_free (muxer->widget_actions_disabled);
 
-  G_OBJECT_CLASS (gtk_action_muxer_parent_class)
-    ->finalize (object);
+  G_OBJECT_CLASS (gtk_action_muxer_parent_class)->finalize (object);
 }
 
 static void
@@ -859,10 +890,6 @@ gtk_action_muxer_get_property (GObject    *object,
       g_value_set_object (value, muxer->widget);
       break;
 
-    case PROP_WIDGET_ACTIONS:
-      g_value_set_boxed (value, muxer->widget_actions);
-      break;
-
     default:
       G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
     }
@@ -886,18 +913,6 @@ gtk_action_muxer_set_property (GObject      *object,
       muxer->widget = g_value_get_object (value);
       break;
 
-    case PROP_WIDGET_ACTIONS:
-      muxer->widget_actions = g_value_get_boxed (value);
-      if (muxer->widget_actions)
-        {
-          int i;
-
-          muxer->widget_actions_enabled = g_new (gboolean, muxer->widget_actions->len);
-          for (i = 0; i < muxer->widget_actions->len; i++)
-            muxer->widget_actions_enabled[i] = TRUE;
-        }
-      break;
-
     default:
       G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
     }
@@ -908,6 +923,7 @@ gtk_action_muxer_init (GtkActionMuxer *muxer)
 {
   muxer->observed_actions = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, gtk_action_muxer_free_action);
   muxer->groups = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, gtk_action_muxer_free_group);
+  muxer->widget_actions_disabled = _gtk_bitmask_new ();
 }
 
 static void
@@ -959,13 +975,6 @@ gtk_action_muxer_class_init (GObjectClass *class)
                                                  G_PARAM_CONSTRUCT_ONLY |
                                                  G_PARAM_STATIC_STRINGS);
 
-  properties[PROP_WIDGET_ACTIONS] = g_param_spec_boxed ("widget-actions", "Widget actions",
-                                                        "Widget actions",
-                                                        G_TYPE_PTR_ARRAY,
-                                                        G_PARAM_READWRITE |
-                                                        G_PARAM_CONSTRUCT_ONLY |
-                                                        G_PARAM_STATIC_STRINGS);
-
   g_object_class_install_properties (class, NUM_PROPERTIES, properties);
 }
 
@@ -1060,17 +1069,14 @@ gtk_action_muxer_remove (GtkActionMuxer *muxer,
 /*< private >
  * gtk_action_muxer_new:
  * @widget: the widget to which the muxer belongs
- * @actions: widget actions
  *
  * Creates a new #GtkActionMuxer.
  */
 GtkActionMuxer *
-gtk_action_muxer_new (GtkWidget *widget,
-                      GPtrArray *actions)
+gtk_action_muxer_new (GtkWidget *widget)
 {
   return g_object_new (GTK_TYPE_ACTION_MUXER,
                        "widget", widget,
-                       "widget-actions", actions,
                        NULL);
 }
 

diff --git a/gtk/gtkactionmuxerprivate.h b/gtk/gtkactionmuxerprivate.h
index a37d30cf7e07c661fcc14709ca5e1e7e22aa6104..0c5670c6f88eb00aefd41aa331573d383e2cff3c 100644 (file)
--- a/gtk/gtkactionmuxerprivate.h
+++ b/gtk/gtkactionmuxerprivate.h
@@ -31,7 +31,13 @@ G_BEGIN_DECLS
 #define GTK_IS_ACTION_MUXER(inst)                           (G_TYPE_CHECK_INSTANCE_TYPE ((inst),                     \
                                                              GTK_TYPE_ACTION_MUXER))
 
-typedef struct {
+typedef struct _GtkWidgetAction GtkWidgetAction;
+typedef struct _GtkActionMuxer GtkActionMuxer;
+
+struct _GtkWidgetAction
+{
+  GtkWidgetAction *next;
+
   char *name;
   GType owner;
 
@@ -40,13 +46,10 @@ typedef struct {
 
   const GVariantType *state_type;
   GParamSpec *pspec;
-} GtkWidgetAction;
-
-typedef struct _GtkActionMuxer                              GtkActionMuxer;
+};
 
 GType                   gtk_action_muxer_get_type                       (void);
-GtkActionMuxer *        gtk_action_muxer_new                            (GtkWidget      *widget,
-                                                                         GPtrArray      *actions);
+GtkActionMuxer *        gtk_action_muxer_new                            (GtkWidget      *widget);
 
 void                    gtk_action_muxer_insert                         (GtkActionMuxer *muxer,
                                                                          const gchar    *prefix,

diff --git a/gtk/gtkapplication.c b/gtk/gtkapplication.c
index 768915c807f9508ccda486c7ed9deb24175669aa..99eae4248a245fc5449a28ec557047d8ec326aa7 100644 (file)
--- a/gtk/gtkapplication.c
+++ b/gtk/gtkapplication.c
@@ -402,7 +402,7 @@ gtk_application_init (GtkApplication *application)
 {
   GtkApplicationPrivate *priv = gtk_application_get_instance_private (application);
 
-  priv->muxer = gtk_action_muxer_new (NULL, NULL);
+  priv->muxer = gtk_action_muxer_new (NULL);
 
   priv->accels = gtk_application_accels_new ();
 }

diff --git a/gtk/gtkwidget.c b/gtk/gtkwidget.c
index e4e610b7e6b3d1cd036f7b5298897cea355e5c8c..6d25c73357490f445374fb2e6521dc67081fe1b1 100644 (file)
--- a/gtk/gtkwidget.c
+++ b/gtk/gtkwidget.c
@@ -11086,7 +11086,7 @@ _gtk_widget_get_action_muxer (GtkWidget *widget,
 
   if (create || priv->actions)
     {
-      muxer = gtk_action_muxer_new (widget, priv->actions);
+      muxer = gtk_action_muxer_new (widget);
       g_object_set_qdata_full (G_OBJECT (widget),
                                quark_action_muxer,
                                muxer,
@@ -12664,29 +12664,12 @@ gtk_widget_class_add_action (GtkWidgetClass  *widget_class,
 {
   GtkWidgetClassPrivate *priv = widget_class->priv;
 
-  if (priv->actions == NULL)
-    priv->actions = g_ptr_array_new ();
-  else
-    {
-      GtkWidgetClass *parent_class = GTK_WIDGET_CLASS (g_type_class_peek (g_type_parent (G_TYPE_FROM_CLASS (widget_class))));
-      GtkWidgetClassPrivate *parent_priv = parent_class->priv;
-      GPtrArray *parent_actions = parent_priv->actions;
-
-      if (priv->actions == parent_actions)
-        {
-          int i;
-
-          priv->actions = g_ptr_array_new ();
-          for (i = 0; i < parent_actions->len; i++)
-            g_ptr_array_add (priv->actions, g_ptr_array_index (parent_actions, i));
-        }
-    }
-
   GTK_NOTE(ACTIONS, g_message ("%sClass: Adding %s action\n",
                                g_type_name (G_TYPE_FROM_CLASS (widget_class)),
                                action->name));
 
-  g_ptr_array_add (priv->actions, action);
+  action->next = priv->actions;
+  priv->actions = action;
 }
 
 /*
@@ -12866,11 +12849,13 @@ gtk_widget_class_query_action (GtkWidgetClass      *widget_class,
                                const char         **property_name)
 {
   GtkWidgetClassPrivate *priv = widget_class->priv;
+  GtkWidgetAction *action = priv->actions;
 
-  if (priv->actions && index_ < priv->actions->len)
-    {
-      GtkWidgetAction *action = g_ptr_array_index (priv->actions, index_);
+  for (; index_ > 0 && action != NULL; index_--)
+    action = action->next;
 
+  if (action != NULL && index_ == 0)
+    {
       *owner = action->owner;
       *action_name = action->name;
       *parameter_type = action->parameter_type;

diff --git a/testsuite/gtk/action.c b/testsuite/gtk/action.c
index 23a78fa23e3c6f5b330d800d4c81dcbecb1b9d14..1812d45ba5cd568ee1b206c6de41bad4cd378a7e 100644 (file)
--- a/testsuite/gtk/action.c
+++ b/testsuite/gtk/action.c
@@ -358,15 +358,15 @@ test_introspection (void)
     const char *params;
     const char *property;
   } expected[] = {
-    { GTK_TYPE_TEXT, "text.undo", NULL, NULL },
-    { GTK_TYPE_TEXT, "text.redo", NULL, NULL },
-    { GTK_TYPE_TEXT, "clipboard.cut", NULL, NULL },
-    { GTK_TYPE_TEXT, "clipboard.copy", NULL, NULL },
-    { GTK_TYPE_TEXT, "clipboard.paste", NULL, NULL },
-    { GTK_TYPE_TEXT, "selection.delete", NULL, NULL },
-    { GTK_TYPE_TEXT, "selection.select-all", NULL, NULL },
-    { GTK_TYPE_TEXT, "misc.insert-emoji", NULL, NULL },
     { GTK_TYPE_TEXT, "misc.toggle-visibility", NULL, "visibility" },
+    { GTK_TYPE_TEXT, "misc.insert-emoji", NULL, NULL },
+    { GTK_TYPE_TEXT, "selection.select-all", NULL, NULL },
+    { GTK_TYPE_TEXT, "selection.delete", NULL, NULL },
+    { GTK_TYPE_TEXT, "clipboard.paste", NULL, NULL },
+    { GTK_TYPE_TEXT, "clipboard.copy", NULL, NULL },
+    { GTK_TYPE_TEXT, "clipboard.cut", NULL, NULL },
+    { GTK_TYPE_TEXT, "text.redo", NULL, NULL },
+    { GTK_TYPE_TEXT, "text.undo", NULL, NULL },
   };
 
   i = 0;